Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves #10 : Making testing-framework module #11

Open
wants to merge 3 commits into
base: public
Choose a base branch
from
Open

Conversation

LarsSchaaf
Copy link

Done:

  • Move all files to a subfolder testingframework, such that it could be imported as from testingframework.utilities import *, note that testing-framework would not be valid subdirectory name.
  • Adding a setup.py file, something like below.

Still to do: Implementing versioneer

@LarsSchaaf LarsSchaaf requested a review from stenczelt July 11, 2021 23:12
@LarsSchaaf LarsSchaaf linked an issue Jul 11, 2021 that may be closed by this pull request
@gabor1
Copy link
Contributor

gabor1 commented Jul 11, 2021

Do you need all these eggs checked in?

@LarsSchaaf
Copy link
Author

Sorry my mistake! I'll create new pull request

@stenczelt
Copy link
Member

great work @LarsSchaaf, I will take a look later today

@stenczelt
Copy link
Member

Sorry my mistake! I'll create new pull request

don't worry just remove from this one with a commit or force push a modified version to the same commit, no need for a new pull request :)

@stenczelt
Copy link
Member

@LarsSchaaf do you actually need pytest-runner for the testing of this module? I am under the impression that pytest itself as installed gets everything done and pytest-runner is deprecated/will be soon forgotten.

see:

@stenczelt
Copy link
Member

wait are there even unittests? the tests we have are the science tests here, not ones that pytest would be working with. I suppose that dependency can be deleted as a whole.

version=version["__version__"],
description=DESCRIPTION,
long_description=open("README.md").read(),
install_requires=["scipy", "numpy", "matplotlib", "pandas>=0.21.0", "scipy",],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • scipy is duplicated
  • ase is not there
  • pandas is only used in the demo notebook, not the package
  • anything else?

there are optional dependencies, that I wouldn't want to install unless needed, like torch, torchani, pyjulip, etc.

tests_require=["pytest",],
author=AUTHOR,
author_email=AUTHOR_EMAIL,
package_data={"": ["data/*", "calib/data/*"],},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these directories don't seem to exist

from setuptools import setup, find_packages

PACKAGENAME = "testingframework"
DESCRIPTION = "Module for testing various interatomic potentials"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Module" or "Package" ?


setup(
name=PACKAGENAME,
packages=find_packages(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no __init__.py in the testingframework directory, that may be useful there

packages=find_packages(),
version=version["__version__"],
description=DESCRIPTION,
long_description=open("README.md").read(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we read this in above just like the other parts? just being a little petty here, but with ... as is nicer for this, even though we can be pretty sure that the file will be closed properly like this as well.

@stenczelt
Copy link
Member

Finally, @LarsSchaaf can you confirm that you have installed this and used the testing of a model correctly and this reproduces the behaviour as before, or that you can document how the testing should be modified now?

Once all these are done, I think it would be useful if you asked @jameskermode and @bernstei to take a look as well, because they are the longest standing users here perhaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Making testing-framework module
3 participants